Skip to content

Frontend hardening (8 findings)#16

Merged
JMRussas merged 1 commit intomainfrom
fix/frontend-hardening
Mar 5, 2026
Merged

Frontend hardening (8 findings)#16
JMRussas merged 1 commit intomainfrom
fix/frontend-hardening

Conversation

@JMRussas
Copy link
Owner

@JMRussas JMRussas commented Mar 4, 2026 •
edited
Loading

Summary

  • SSE reconnect fix (Change license from MIT to AGPL v3 #10): Close EventSource on error, fetch fresh token, exponential backoff (1-30s), max 5 retries. Fixes infinite loop caused by expired short-lived tokens.
  • useFetch cancellation (#29): cancelledRef guard prevents setState after unmount.
  • OIDC StrictMode (#30): processedRef prevents double-fire in React StrictMode dev mode.
  • RequireRole guard (#31): New route-level component wrapping /admin and /analytics. Removed inline role check from Analytics page. Replaced alert() with inline error in Admin page.
  • Responsive layout (#32): @media queries at 1023px (tablet: 2-col grids) and 767px (mobile: stacked layout, horizontal nav).
  • Accessibility (#33): Modal role="dialog" + aria-modal + aria-labelledby + focus trap + body scroll lock. Progress bars with role="progressbar" + aria attributes. Form labels with htmlFor/id. Nav aria-label.
  • CSS fix (#46): var(--hover) → var(--border) in .oauth-btn:hover.
  • authFetch export (#47): Export authFetch from client.ts, rewrite exportProject to use it instead of raw fetch.

Self-review fix

  • retryCountRef not reset on projectId change in useSSE — if SSE to project A failed N times, navigating to project B would carry over the retry count, reducing available retries.

Test plan

  • 157 frontend tests pass (up from 137 — 20 new/updated tests)
  • TypeScript compiles clean (npx tsc --noEmit)
  • New RequireRole.test.tsx: 4 tests (correct role, wrong role, no user, custom fallback)
  • Updated useSSE.test.ts: 3 new tests (onerror closes source, reconnect with fresh token, retry count reset on open)
  • Updated useFetch.test.ts: cancellation guard test
  • Updated Modal.test.tsx: ARIA attributes test, close button aria-label test
  • Updated Analytics.test.tsx: removed dead inline role guard test (now handled by RequireRole)
  • Manual: verify SSE reconnect in browser devtools (disconnect network, verify backoff + fresh token)
  • Manual: verify responsive layout at 768px and 480px widths
  • Manual: verify Modal keyboard navigation (Tab cycling, Escape, focus restore)

Generated by Claude Code · Claude Opus 4.6

@JMRussas @claude
Frontend hardening (8 findings)
714dfae 
- Fix SSE reconnect infinite loop: exponential backoff, fresh tokens, max 5 retries (#10)
- Add useFetch cancellation guard to prevent setState after unmount (#29)
- Fix OIDC StrictMode double-fire with processedRef guard (#30)
- Add RequireRole route guard, wire admin/analytics routes (#31)
- Add responsive layout at tablet (1023px) and mobile (767px) breakpoints (#32)
- Add ARIA: Modal focus trap/save/restore, progressbar roles, form labels (#33)
- Fix var(--hover) → var(--border) in .oauth-btn:hover (#46)
- Export authFetch, rewrite exportProject to use it (#47)

Self-review fix: reset retryCountRef on projectId change in useSSE.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@JMRussas
Copy link
Owner Author

JMRussas commented Mar 4, 2026

Self-review defect: retryCountRef not reset on projectId change

File: frontend/src/hooks/useSSE.ts:49

What the defect was: retryCountRef is a React ref that persists across effect re-runs. When the user navigated from project A (where SSE had failed N times) to project B, the retry count carried over, giving project B only MAX_RETRIES - N retries instead of the full 5.

How it was fixed: Added retryCountRef.current = 0 at the start of the effect body (before calling connect()), so each new projectId starts with a fresh retry budget.

Why the original code was wrong: The onopen handler already resets retryCountRef on successful connection, but that doesn't cover the case where the first connection to a new project also fails — the stale count from the previous project would persist.

@JMRussas JMRussas merged commit f95f7e7 into main Mar 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@JMRussas